-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use multierror instead of errors.Join #153
Conversation
just a small fix to help with backwards compatibility for projects stuck on go < 1.20. This just postpones use of errors.Join in favour of multierror.Append. No functional changes.
Thanks @bharling - I'm in support of this! |
Aside from acceptance tests failing, could you give me more context about why this is needed?
Merging this patch means I can no longer use the Go 1.20+ feature until you finish upgrading, right? |
@minamijoyo I work closely with @bharling and can offer some context...
I completely understand; it may not be worth catering to a niche use case. Nonetheless, here's some thoughts to consider... While it's beyond your original intended use case -- and perhaps not worth supporting -- I do think there's some utility to using
Again, I completely understand if you don't want to support
That all said, I defer to @minamijoyo :) And, a big disclaimer: I haven't looked into the acceptance test failures; I value a solution to #129 over < Go 1.20 support. I'm assuming @bharling 's intent is that |
Completely understand and echo the points raised by @mdb above - this is essentially a fairly selfish PR on our part just to unblock our usage of tfmigrate in our current project, totally appreciate it's not helping to advance tfmigrate itself any. I will take a look into the tests asap though regardless |
I do care about compatibility as a CLI but not as a library. That said, I don't intend to hide everything in the internal package as terraform does, so it's ok to use tfmigrate as a library at your own risk with the understanding that breaking changes could be merged at any time. Having that in mind, my answer would be to upgrade your Go version to the latest in general. Even though you are the author of patch #129 and using the multierror looks relatively harmless, I can accept it as a temporary workaround. If you still wish to do so, could you provide the following information?
Without them, unblocking your project will block this project. It's hard to accept without any timeline. |
Thanks very much for taking the time to reply - on reflection I think the best option for tfmigrate would be to close this PR and for us to focus on resolving the upgrade issues our side. I don't want to block progress on tfmigrate nor create a dependency on our own work to revert the changes I'm proposing here. Thanks again for looking into this and apologies for the waste of time! |
I appreciate your understanding, and It's also good to hear your use case 👍 |
just a small fix to help with backwards compatibility for projects stuck on go < 1.20. This just postpones use of errors.Join in favour of multierror.Append. No functional changes.